-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implemented stable image shrink #513
Conversation
// Path: packages/image-shrink/src/helper | ||
export { memoize, memoKeySerializer } from './helper/memoize' | ||
|
||
export { shrinkFile, type TSetting } from './utils/shrinkFile' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the only thing we should export is this one.
@@ -0,0 +1,4 @@ | |||
export const allowLayers = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the folder name (constans)
} | ||
|
||
const squareSupported = await squareTest(testSquareSide, testSquareSide) | ||
const dimensionSupported = await dimensionTest(testDimension, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably it's better to use Promise.all here
|
||
return isTestPass | ||
} catch (e) { | ||
new Error(`Failed to test for max canvas size of ${width}x${height}.`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error isn't throwed or returned. Probably it should be console.log
or console.error
return new Promise((resolve, reject) => { | ||
replaceIccProfile(inputFile, []) | ||
.then((file: Blob) => { | ||
console.log({ file }, typeof file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove console log
image: File | Blob | string | ||
): Promise<HTMLImageElement> => { | ||
if (image.src) { | ||
return processImage(image) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove if block
import { imageLoader } from '../image/imageLoader' | ||
|
||
export const stripIccProfile = (inputFile: File): Promise<HTMLImageElement> => { | ||
return new Promise((resolve, reject) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactor promises chain
e5322b6
to
de4850d
Compare
Description
Checklist